-
Notifications
You must be signed in to change notification settings - Fork 5.7k
[JEWEL-1026] Add support for fractional palette indices #3383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| public val purple: List<Color>, | ||
| public val teal: List<Color>, | ||
| public val rawMap: Map<String, Color>, | ||
| public val fractionalMap: Map<String, Color> = emptyMap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would store all colours in just one map, given the keys are strings anyway, and have the code understand that non-fractional colours simply have a fractional part of zero. That way you can also look up grayOrNull(1, 0) to get Gray1, but more importantly, code does not have to access two separate maps to find a colour.
6fc01f5 to
f560fa9
Compare
| - sf:PALETTE_KEY_PREFIX:java.lang.String | ||
| - <init>(java.util.List,java.util.List,java.util.List,java.util.List,java.util.List,java.util.List,java.util.List,java.util.List,java.util.Map):V | ||
| - f:blue-vNxB06k(I):J | ||
| - f:blueOrNull-ijrfgN4(I):androidx.compose.ui.graphics.Color |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid breaking binary changes, we need to keep the old APIs, but deprecate them with DeprecationLevel.HIDDEN
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so that means that I should just add new methods (grayOrNull(Int, Int) etc) and also keep the old ones (grayOrNull(Int) etc)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for example:
@Deprecated("Use variant with fractional parameter", level = DeprecationLevel.HIDDEN)
public fun grayOrNull(index: Int): Color? = gray.getOrNull(index - 1)
public fun grayOrNull(index: Int, fractional: Int = 0): Color? =
if (fractional == 0) {
gray.getOrNull(index - 1)
} else {
rawMap["$PALETTE_KEY_PREFIX.Gray$index.$fractional"]
}The hidden symbols will still disappear from the dumps, but they will be included in the binary and invisible from code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes — @faogustavo has helpfully listed a legend on how to read the dumps here:
* - marked with @ApiStatus.Experimental.
b - synthetic (as in binary visible). Members with both ACC_SYNTHETIC and ACC_BRIDGE are not included in the dump.
p - protected. Nothing for public because it's the default. private and package-private members are not included.
s - static.
@ - annotation.
e - enum.
f - final.
a - abstract.
c - class. Nothing for interface because it should be the default.
So, you're deprecating with HIDDEN level the existing blueOrNull(Int) — that's why it gets the b added to its existing f (DeprecationLevel.HIDDEN is a clever trick that (ab)uses the concept of a synthetic method in JVM bytecode). This is expected.
Next, you added blueOrNull(Int, Int = 0) where the second parameter has a default value; this is reflected in JVM bytecode as:
- A
f:blueOrNull-6MYuD4A(I,I)with two Int parameters, both non-optional - A
f:blueOrNull-6MYuD4A$default(...)that is generated by the compiler to implement the default parameter behaviour
The weird -6MYuD4A suffixes are also added by the Kotlin Compiler; it's called mangling and you can read more about it here: https://ncorti.com/blog/name-mangling-in-kotlin
f560fa9 to
3407a6f
Compare
|
Ready to merge |


This PR adds support for fractional palette indices (e.g., ColorPalette.Gray1.25) in ThemeColorPalette and updates parsing of fractional indices colors in BridgeColorPalette. Previously, only integer indices were supported.
Release notes
New features
ThemeColorPalette